Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

361 taskmeasureinstrument handling with document upload and status change #410

Conversation

ChristopherSpelt
Copy link
Contributor

@ChristopherSpelt ChristopherSpelt commented Dec 3, 2024

Description

What is added

  1. Added additional statuses (todo, in progress, in review, done, not implemented). Note that this also means adding additional colours to the status bar icon in the measures in the requirements overview. Since the status bar only has a limited set of colours, this is now suboptimal visually. The current colours are aligned with Ruben.
  2. Possibility to add URI's to files. The input field is not checked, so any user input will be stored in the system card under the corresponding measure in the field links.
  3. Possibility to upload files. NOTE: it was decided to REMOVE the file list when adding files, before saved is pressed.
  4. Possibility to download an uploaded file from the object storage.
  5. Possibility to remove the uploaded files.
  6. Added tests for the new ObjectStorageService
  7. Added tests for the new GET and DELETE endpoints for the files

What is not added

  1. Automatically render an icon according to the file extension. For example a PDF logo for a PDF file.
  2. A progress bar for uploading files.

What still needs to be done

  1. (very OPTIONAL) remove the coupling of the ObjectStorageService with Minio, maybe by using protocols or something.
  2. Code review
  3. Code review + merge of the PR in the infra repo: Add env vars for minio service ai-validation-infra#85
  4. Check if everything works on staging
  5. Merge the PR

Estimation of leftover points: 1 or 2.

If things don't work properly

-> Berry probably knows most about the Minio side of things
-> Robbert U knows probably most about the way the UI of the form is implemented

How to tests things locally

Use a local instance of Minio with the settings which are specified in the core settings in AMT.

Resolves #361

Checklist

Please check all the boxes that apply to this pull request using "x":

  • I have tested the changes locally and verified that they work as expected.
  • I have followed the project's coding conventions and style guidelines.
  • I have rebased my branch onto the latest commit of the main branch.
  • I have squashed or reorganized my commits into logical units.
  • I have read, understood and agree to the Developer Certificate of Origin, which this project utilizes.

@ChristopherSpelt ChristopherSpelt linked an issue Dec 3, 2024 that may be closed by this pull request
9 tasks
@ChristopherSpelt ChristopherSpelt force-pushed the 361-taskmeasureinstrument-handling-with-document-upload-and-status-change branch 5 times, most recently from 4fa03e5 to 41aa229 Compare December 10, 2024 10:13
@ChristopherSpelt ChristopherSpelt force-pushed the 361-taskmeasureinstrument-handling-with-document-upload-and-status-change branch 7 times, most recently from af1f9bb to d932fa6 Compare December 11, 2024 15:24
@ChristopherSpelt ChristopherSpelt marked this pull request as ready for review December 11, 2024 15:57
@ChristopherSpelt ChristopherSpelt requested a review from a team as a code owner December 11, 2024 15:57
@laurensWe laurensWe force-pushed the 361-taskmeasureinstrument-handling-with-document-upload-and-status-change branch from d932fa6 to a759af3 Compare December 13, 2024 10:48
laurensWe
laurensWe previously approved these changes Dec 13, 2024
amt/locale/nl_NL/LC_MESSAGES/messages.po Outdated Show resolved Hide resolved
amt/locale/nl_NL/LC_MESSAGES/messages.po Outdated Show resolved Hide resolved
@laurensWe laurensWe force-pushed the 361-taskmeasureinstrument-handling-with-document-upload-and-status-change branch 3 times, most recently from 9708298 to 434fa23 Compare December 17, 2024 14:56
@laurensWe laurensWe force-pushed the 361-taskmeasureinstrument-handling-with-document-upload-and-status-change branch from 434fa23 to b9d2d42 Compare December 17, 2024 15:25
ravimeijerrig
ravimeijerrig previously approved these changes Dec 17, 2024
Copy link
Contributor

@ravimeijerrig ravimeijerrig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

Copy link
Contributor

@ravimeijerrig ravimeijerrig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

@laurensWe laurensWe enabled auto-merge December 17, 2024 15:45
Copy link

sonarcloud bot commented Dec 17, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
84.3% Coverage on New Code (required ≥ 95%)

See analysis details on SonarQube Cloud

@laurensWe laurensWe merged commit de2c644 into main Dec 17, 2024
14 of 15 checks passed
@laurensWe laurensWe deleted the 361-taskmeasureinstrument-handling-with-document-upload-and-status-change branch December 17, 2024 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Task/Measure/Tool handling with document upload and status change
3 participants